Skip to content

[hono] Add support for JDBC device registry#164

Merged
sophokles73 merged 1 commit intoeclipse-packages:masterfrom
lhotari:lh-hono-device-registry-jdbc
Dec 17, 2020
Merged

[hono] Add support for JDBC device registry#164
sophokles73 merged 1 commit intoeclipse-packages:masterfrom
lhotari:lh-hono-device-registry-jdbc

Conversation

@lhotari
Copy link
Contributor

@lhotari lhotari commented Nov 23, 2020

Motivation

Add support for using JDBC based device & tenant registry in the Hono Helm chart.
JDBC based device registries will be part of the upcoming Hono 1.5.0 release.

Fixes #163

Dependencies

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch 2 times, most recently from 3577b3a to 821f985 Compare November 23, 2020 16:35
@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch 2 times, most recently from c47c934 to ba67aa6 Compare November 23, 2020 16:50
Copy link
Contributor

@ctron ctron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. But I will leave it for others to comment as well.

@kaniyan
Copy link
Contributor

kaniyan commented Nov 24, 2020

Looks good to me. With this PR, we can use this chart to deploy the JDBC device registry and connect to an already existing JDBC database. In order to provide an out of the box experience, I think its good to have an option to install a JDBC database instance on the fly. And the JDBC registry can connect to it as we do in the MongoDB device registry. I remember that in some meeting we talked about using a H2 database instance for that. Also it can be done as a separate PR.

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from ecaef9e to 2fd9e33 Compare November 24, 2020 11:49
username:
# The password to use for authenticating to the database
password:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to have default configuration for using an in memory H2 DB so that you can simply start the registry and use it, without the need to set a lot of configuration properties ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

point taken, however setting up h2 using a persistent file would require quite some changes to the helm chart. Would in-memory h2 be ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, we can still evolve from there ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I get the issue with persistence. That would be one pvc and a volume mapping in case of h2. Could be a simple feature flag to enable that.

@calohmn
Copy link
Contributor

calohmn commented Nov 25, 2020

@lhotari Can you rebase and resolve the conflict?
EDIT: seeing that this PR has to wait for the Hono 1.5.0 release, you can also do that later, as there will probably be some more updates to the Chart.yaml until then.

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from 2fd9e33 to 550ae06 Compare November 25, 2020 14:40
@lhotari
Copy link
Contributor Author

lhotari commented Nov 25, 2020

Can you rebase and resolve the conflict?

@calohmn yes, that's done now.

@calohmn calohmn added the Hono label Dec 15, 2020
@sophokles73
Copy link
Contributor

hi @lhotari we have released Hono 1.5.0 today and it would be cool if we could add your changes now to the Helm chart. Would you mind rebasing your PR and bumping the chart version?

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from 550ae06 to 85d4287 Compare December 17, 2020 12:18
@lhotari
Copy link
Contributor Author

lhotari commented Dec 17, 2020

hi @lhotari we have released Hono 1.5.0 today and it would be cool if we could add your changes now to the Helm chart. Would you mind rebasing your PR and bumping the chart version?

@sophokles73 I have rebased my PR and bumped the chart version to 1.4.17 . Is that correct?

@sophokles73 sophokles73 merged commit 4ec66de into eclipse-packages:master Dec 17, 2020
@sophokles73
Copy link
Contributor

@lhotari thanks for contributing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[eclipse/hono#2159] Add support to deploy JDBC based device registry

5 participants